Skip to content
This repository was archived by the owner on Jan 24, 2018. It is now read-only.

Adds setActiveLink method.#51

Open
pedrocorreia wants to merge 1 commit intoPrinzhorn:masterfrom
pedrocorreia:master
Open

Adds setActiveLink method.#51
pedrocorreia wants to merge 1 commit intoPrinzhorn:masterfrom
pedrocorreia:master

Conversation

@pedrocorreia
Copy link
Copy Markdown

Hi, again!
I've added a method to add a class to the currently active link. It defaults to active but it's also available in the options object as activeClass.

I've also added a menuSelector option to identify the menu html element so it's easier to target and avoid complex dom queries. This defaults to '.skrollr-menu'.

skrollr.menu.init(s, {
    activeClass: 'active',
    menuSelector: '.skrollr-menu'
});

The previous problem was because i use indentation with spaces and your files seem to be indented with tabs. I've set my changes to your standard.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think a menu selector is needed. There can be links all over the website, not just inside a menu.

@Prinzhorn
Copy link
Copy Markdown
Owner

I added some comments. But basically I kind of doubt the usefulness. Simply adding/removing a class when the link is clicked can easily be done outside of skrollr. The tricky part is updating the state as you scroll, see #6

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants